Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds claim api for vg #4708

Merged
merged 2 commits into from
Mar 9, 2020
Merged

Adds claim api for vg #4708

merged 2 commits into from
Mar 9, 2020

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented Feb 21, 2020 •
edited by LaurenWags
Loading

Resolves brave/brave-browser#8354

Submitter Checklist:

Test Plan:

Plan 1:

  • enable rewards
  • claim ad grant and ugp
  • connect to KYC uphold account
  • make sure that ad grant was transferred to uphold while ugp stayed as VG

Plan 1:

  • enable rewards
  • connect to KYC uphold account
  • disconnect uphold
  • claim ad grant and ugp
  • reconnect to uphold
  • make sure that ad grant was transferred to uphold while ugp stayed as VG

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@NejcZdovc NejcZdovc self-assigned this Feb 21, 2020
@NejcZdovc NejcZdovc force-pushed the claim-vg branch 2 times, most recently from c32b001 to 60282bb Compare February 26, 2020 19:51
This was referenced Feb 27, 2020
Closed
Closed
@NejcZdovc NejcZdovc force-pushed the claim-vg branch 3 times, most recently from df535d3 to 60cbbf9 Compare March 3, 2020 08:06
@NejcZdovc NejcZdovc added this to the 1.7.x - Nightly milestone Mar 3, 2020
@NejcZdovc NejcZdovc marked this pull request as ready for review March 3, 2020 09:17
@NejcZdovc NejcZdovc requested a review from a team March 3, 2020 09:17
@NejcZdovc NejcZdovc force-pushed the claim-vg branch 2 times, most recently from 0055df7 to e5afc73 Compare March 3, 2020 09:23
zenparsing
zenparsing reviewed Mar 5, 2020
View reviewed changes
components/brave_rewards/browser/rewards_service.cc Show resolved Hide resolved
vendor/bat-native-ledger/src/bat/ledger/internal/database/database_unblinded_token.cc Outdated Show resolved Hide resolved
vendor/bat-native-ledger/src/bat/ledger/internal/promotion/promotion_transfer.cc Outdated Show resolved Hide resolved
vendor/bat-native-ledger/src/bat/ledger/internal/uphold/uphold_wallet.cc Show resolved Hide resolved
vendor/bat-native-ledger/src/bat/ledger/internal/promotion/promotion_util.cc Outdated Show resolved Hide resolved
vendor/bat-native-ledger/src/bat/ledger/internal/promotion/promotion_transfer.cc

base::Value token(base::Value::Type::DICTIONARY);
bool success = false;
if (ledger::is_testing) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this logic is also in contribution_util.cc, I wonder if there might be a way to make this a single call, either by passing in ledger::is_testing or something similar.

@NejcZdovc NejcZdovc force-pushed the claim-vg branch 2 times, most recently from c67f120 to ba3a76c Compare March 6, 2020 06:58
@NejcZdovc
Copy link
Contributor Author

everything passed except iOS, restarting iOS

@NejcZdovc NejcZdovc added CI/skip-android Do not run CI builds for Android CI/skip-linux CI/skip-macos-x64 Do not run CI builds for macOS x64 labels Mar 6, 2020
@NejcZdovc NejcZdovc removed CI/skip-android Do not run CI builds for Android CI/skip-linux CI/skip-macos-x64 Do not run CI builds for macOS x64 labels Mar 9, 2020
@NejcZdovc
Copy link
Contributor Author

NejcZdovc commented Mar 9, 2020 •
edited
Loading

CI failed on unrelated changes (audit-depos)

@NejcZdovc NejcZdovc merged commit eb54c79 into master Mar 9, 2020
@NejcZdovc NejcZdovc deleted the claim-vg branch March 9, 2020 15:29
@LaurenWags
Copy link
Member

Verified using

Brave 1.7.52 Chromium: 80.0.3987.132 (Official Build) nightly (64-bit)
Revision fcea73228632975e052eb90fcf6cd1752d3b42b4-refs/branch-heads/3987@{#974}
OS macOS Version 10.14.6 (Build 18G3020)

Verified the two test plans from the above description and 3 other scenarios. All are detailed below with steps.

PASS - Test plan 1 from PR description - #4708:

  1. Install 1.7.52 and enable rewards (staging)
  2. Restored a wallet with both ad and ugp grant
  3. claim ad grant and ugp grant
  4. connected a KYC'd uphold sandbox account
  5. Confirmed that ad grant was transferred to uphold while ugp stayed as VG
[ RESPONSE - OnTransferAnonToExternalWallet ]
> time: 1583855549
> result: Success

[ RESPONSE - DeleteTokens ]
> time: 1583855549
> result: Success

Screen Shot 2020-03-10 at 11 54 21 AM

Screen Shot 2020-03-10 at 11 54 06 AM

PASS Test plan 2 from PR description - #4708:

  1. Install 1.7.52 and enable rewards (staging)
  2. Restore a wallet with both ad and ugp grants (do not claim yet)
  3. connect to KYC'd uphold account
  4. disconnect uphold
  5. claim ad and ugp grants
  6. Reconnect to Uphold
  7. Confirmed that ad grant was transferred to uphold while ugp stayed as VG
[ RESPONSE - OnTransferAnonToExternalWallet ]
> time: 1583868941
> result: Success

[ RESPONSE - DeleteTokens ]
> time: 1583869920
> result: Success

claiming while disconnected:
Screen Shot 2020-03-10 at 3 40 10 PM

reconnecting and ad grant is transferred to uphold:
Screen Shot 2020-03-10 at 3 53 26 PM
Screen Shot 2020-03-10 at 3 53 33 PM

PASS New User Scenario

  1. Install Nightly 1.7.52, launch with flags for staging
  2. Enable BR
  3. Restore a wallet with an ad grant
  4. Claim ad grant
  5. Start wallet verification process (important, start with a non-kyc'd wallet and complete KYC process)
  6. Get signed up acct KYC'd (may need to request in channel)
  7. Launch again (with logging info)
  8. Confirm you get notification that your KYC is complete
  9. Confirm in logs that ad grant BAT is sent to Uphold - confirmed OnTransferAnonToExternalWallet and DeleteTokens were successful.
[ RESPONSE - OnTransferAnonToExternalWallet ]
> time: 1583853683
> result: Success

[ RESPONSE - DeleteTokens ]
> time: 1583853684
> result: Success

Screen Shot 2020-03-10 at 11 22 27 AM

Screen Shot 2020-03-10 at 11 30 38 AM

Screen Shot 2020-03-10 at 11 23 12 AM

PASS - Upgrade test 1 (partially spend ad grant)

  1. Install 1.4.x (staging env).
  2. Restore a wallet with an ad grant and ugp grant (do not claim yet).
  3. Connect KYCd wallet.
  4. Claim ad grant first.
  5. Claim ugp grant second.
  6. Tip a KYC'd publisher (such as 3zsistemi.si) 1 BAT.
  7. Upgrade to Nightly 1.7.52.
  8. Confirm remaining ad grant is moved to Uphold.
  9. Confirm UGP grant remains VG.
[ RESPONSE - OnTransferAnonToExternalWallet ]
> time: 1583857710
> result: Success

[ RESPONSE - DeleteTokens ]
> time: 1583857711
> result: Success

Screen Shot 2020-03-10 at 12 30 29 PM

Screen Shot 2020-03-10 at 12 30 41 PM

PASS - Upgrade test 2 (no spending of ad grant)

  1. Install 1.4.x (staging env).
  2. Restore a wallet with an ad grant and ugp grant (do not claim yet).
  3. Connect KYCd wallet.
  4. Claim ad grant.
  5. Upgrade to Nightly 1.7.52.
  6. Confirm entire ad grant is moved to Uphold.
[ RESPONSE - OnTransferAnonToExternalWallet ]
> time: 1583858357
> result: Success

[ RESPONSE - DeleteTokens ]
> time: 1583858358
> result: Success

Screen Shot 2020-03-10 at 12 40 24 PM

Screen Shot 2020-03-10 at 12 40 43 PM

NejcZdovc added a commit that referenced this pull request Mar 12, 2020
@NejcZdovc
Merge pull request #4708 from brave/claim-vg
64f6613 
Adds claim api for vg
@NejcZdovc NejcZdovc mentioned this pull request Mar 13, 2020
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zenparsing zenparsing zenparsing approved these changes

Assignees

@NejcZdovc NejcZdovc

Labels
feature/rewards
Projects
None yet
Milestone
1.7.x - Release
Development

Successfully merging this pull request may close these issues.

Add claim option for VG
3 participants
@NejcZdovc @LaurenWags @zenparsing